-
Notifications
You must be signed in to change notification settings - Fork 86
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add support for Node.js v16.3.0+ #335
Conversation
lib/readfilecontext.js
Outdated
let offset; | ||
let length; | ||
|
||
if (this.signal?.aborted) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change the syntax to support old Nodejs v12.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ofc! 😁, checked that tests are passing locally for me on v12 now too
e730c27
to
14e6eaa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@tschaub please have a look.
Awesome trick! Thank you very much for stretching the life of mock-fs. |
Thanks for the work on this, @Rugvip. Is there a test that could be added that would cover this? I haven't been experiencing the issue myself, so am not 100% clear on the specifics. |
@tschaub the existing mock-fs main branch fails test on latest Nodejs v16. |
c175af6
to
d7237ca
Compare
Thank you for the contribution, @Rugvip. |
Hoping to fix #332 with this
Large parts of the implementation in
readfilecontext.js
are copied straight from the Node.js internals. I'm assuming that is all fine as it's already called out in the licence file that parts of this project are copied from there.